Skip to content

Conversation

@danbev
Copy link
Member

@danbev danbev commented Feb 3, 2025

This commit replaces the status code number with the httplib StatusCode enum values.

The motivation for this change is to make the code a little more readable.

This commit replaces the status code number with the httplib StatusCode
enum values.

The motivation for this change is to make the code a little more
readable.
@ngxson
Copy link
Collaborator

ngxson commented Feb 3, 2025

IMO this change is not really needed because it makes the code become verbose. IIRC this part of code is adapted from openai python library so it's better to keep the status code as-is, no need to replace with an enum

@ngxson
Copy link
Collaborator

ngxson commented Feb 3, 2025

Correction: I'm talking about the code inside format_error_response, I prefer to keep it as-is (also to decouple it from httplib). The rest is ok to move to httplib enum

Revert changes of using httplib's StatusCode in format_error_response.
Remove duplicate status code setting in server.cpp
Fix mistake when setting status code to 503.
@danbev
Copy link
Member Author

danbev commented Feb 5, 2025

Closing this as PR to avoid wasting reviewers time. I'd been looking into llama-server part of the code base which was new to me, and "fixing/suggesting" things as part of my learning process. But I had not really considered the time reviewers spend on issue like this. I'll be more selective/careful in the future about opening PRs.

@danbev danbev closed this Feb 5, 2025
@danbev danbev deleted the server-status-codes branch August 13, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants